Skip to content

fix(loop-context): refuse a similarity threshold that switches stagnation off - #643

Open
THRISHAL12345 wants to merge 1 commit into
cobusgreyling:mainfrom
THRISHAL12345:fix/breaker-similarity-threshold
Open

THRISHAL12345 wants to merge 1 commit into
cobusgreyling:mainfrom
THRISHAL12345:fix/breaker-similarity-threshold

Conversation

@THRISHAL12345

Copy link
Copy Markdown
Contributor

The problem

loop-context --similarity-threshold 95 is accepted. Someone writing 95 almost certainly means 95%, but the value is a fraction, and calculateSimilarity() never returns more than 1.0. Above 1, no two errors can ever count as "the same error":

4 identical failures in a row      exit  decision
default (0.85)                     2     ESCALATE [stagnation]
--similarity-threshold 1           2     ESCALATE [stagnation]
--similarity-threshold 1.5         0     CONTINUE [ok]
--similarity-threshold 95          0     CONTINUE [ok]
--similarity-threshold Infinity    0     CONTINUE [ok]

At those values:

  • Stagnation never fires. That's the rule for a loop retrying one failure, the textbook Infinite Fix Loop.
  • The repeated-action ("frustration") rule never fires, because it uses the same threshold.
  • --prune stops collapsing repeated errors, so the ledger fed back to the agent grows instead.

Only the no-progress rule (5 consecutive failures by default) and the iteration cap are left to stop it.

The CLI already refuses integer flags that would "silently disable the breaker", as parsePositiveIntFlag's comment puts it. The float check only rejected NaN and values ≤ 0, so the most natural typo got through. The library took any number too: checkCircuitBreaker(ledger, { ...DEFAULT_BREAKER, similarityThreshold: 95 }) failed the same way.

What this changes

CLI: --similarity-threshold takes a fraction in (0, 1]. Anything else exits 1 with the reason, and a whole-number percentage gets a direct suggestion:

loop-context failed: --similarity-threshold must be a fraction greater than 0 and at most 1 (default 0.85);
got 95. Similarity is at most 1.0, so no two errors would ever match and the stagnation rule would never
fire. If you meant 95%, use 0.95.

The help text and the README's flag table say so too. The README was missing this flag entirely.

Library: checkCircuitBreaker, pruneLedger, summarizeAttempts and buildContextInjection now throw on a config that would switch a rule off:

  • a similarity threshold outside (0, 1], NaN included;
  • a count that isn't a positive integer: maxIterations, the three thresholds, tokenBudget, window, maxTraceLines.

A breaker that refuses to start is loud. One that silently never breaks is the failure this package exists to prevent. validateBreakerConfig / validatePruneConfig are exported, so callers can check a config up front.

loop-drill: a config that loop-context rejects is now a failed breaker.config drill with the library's message, not a crash. It's still exit 2, so CI still fails. Four of its tests had used 95 and 0 as a trick to simulate a breaker that fires the wrong rule. Valid configs can't produce that any more, which is the point of this fix. So runBreakerDrills takes an optional breaker function, and those tests pass a stand-in that always answers with one trigger. That keeps the "credit only the rule under test" logic covered without relying on a config the library now refuses.

Why exit 1

Exit 1 is loop-context's documented "error" code (0 continue · 2 escalate · 1 error). The README's control-flow example, loop-context --check … || { …; exit 2; }, stops on any non-zero exit. The existing integer-flag rejections already use 1.

Verification

  • Tests:
    • loop-context goes from 53 to 62: 4 new CLI tests and 5 new library tests. They cover 95, 100, 1.5 and Infinity being rejected with no decision printed, 0, −0.5 and abc being rejected, the boundaries (1, 0.95, 0.5, 0.01 and Number.MIN_VALUE all still trip stagnation), --prune validation, every config-taking function, and when the percentage hint appears.
    • loop-drill is at 58.
    • mcp-server passes 28/28. Its loop_check_breaker uses DEFAULT_BREAKER, so it isn't affected.
  • Every guard is load-bearing. I removed each one in turn and confirmed a test fails:
    • loop-context (10 guards): validation in checkCircuitBreaker, pruneLedger and summarizeAttempts, the upper bound, the lower bound, integers only, the tokenBudget check, the frustrationThreshold check, the CLI flag check, and the hint only for whole-number percentages.
    • loop-drill (2 guards): the rejected-config catch and the stand-in breaker being used.
    • The one guard no test missed was an isFinite check. The range comparison already rejects NaN and Infinity, so I removed it rather than keep dead code.
  • Both required workflows pass locally. ci-validate-gates.sh exits 0 with 340 tests passing and none failing, including the loop-drill dogfood run against this repo's breaker. ci-audit-gates.sh passes with a reference score of 100. On Windows I applied fix: guard loops against prompt injection from untrusted input #641's one-line github-triage.test.mjs path fix for the local run only; it isn't in this PR.

Notes

`--similarity-threshold 95` (meant as 95%) was accepted. Similarity is at
most 1.0, so above 1 no two errors ever match: stagnation and the repeated-
action rule never fire, and pruning stops collapsing repeats. Four identical
failures came back CONTINUE, exit 0, where the default escalates. 1.5 and
Infinity did the same. Only no-progress and the iteration cap were left as
backstops.

The CLI now takes a fraction in (0, 1] and says what to use when the value
looks like a percentage ("If you meant 95%, use 0.95"). The library
validates too: checkCircuitBreaker, pruneLedger, summarizeAttempts and
buildContextInjection throw on a similarity threshold outside (0, 1], or on
a non-positive or fractional count, instead of quietly never escalating.
validateBreakerConfig / validatePruneConfig are exported.

loop-drill reports a config loop-context rejects as a failed breaker.config
drill rather than crashing. Its trigger-attribution tests used 95 and 0 to
fake a breaker that fires the wrong rule; they now pass a stand-in breaker.
@github-actions

Copy link
Copy Markdown
Contributor

This PR changes paths that must run the real validate and audit workflows (tools, patterns, scripts, or CI).

Fork PRs from first-time contributors start with those workflows waiting for approval. A maintainer needs to open the Checks tab and click Approve and run workflows. Until that happens, branch protection will show the PR as blocked even after a review.

Content-only PRs (docs/, examples/, stories/, skills/, root markdown) skip this step — required checks are posted from this workflow instead.

— loop-engineering fork-pr-gate

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant